Skip to content

Conversation

@AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Nov 12, 2024

ref getsentry/sentry-javascript#13317

  1. Add Sentry.init snippets to all relevant node integrations
  2. Add supported versions where missing in node integrations
  3. Add node integration entries for dataloader, knex, and lrumemorizer integrations
  4. Make sure dataloader is not marked as default
  5. Alpha-order and add missing entries to js integrations table

@AbhiPrasad AbhiPrasad requested a review from a team November 12, 2024 17:04
@AbhiPrasad AbhiPrasad self-assigned this Nov 12, 2024
@AbhiPrasad AbhiPrasad requested review from lforst and s1gr1d and removed request for a team November 12, 2024 17:04
@vercel
Copy link

vercel bot commented Nov 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2024 2:17pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2024 2:17pm
develop-docs ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2024 2:17pm

@codecov
Copy link

codecov bot commented Nov 12, 2024

Bundle Report

Changes will increase total bundle size by 1.25kB (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 8.59MB 1.25kB (0.01%) ⬆️
sentry-docs-client-array-push 8.94MB 6 bytes (-0.0%) ⬇️


```JavaScript
Sentry.init({
dsn: "___PUBLIC_DSN___",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would honestly not add the DSN here (and all the other snippets). It is not relevant to the snippet and it will create an ugly banner on top of the snippet if you are not logged in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It also looks like you left it out in some places. Should we do all or nothing? I'd go for "nothing")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah let's go for nothing - I agree!

## Supported Versions

- Connect `^3.0.0`
- `connect`: `^3.0.0`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No init snippet here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I need to add them for all the server frameworks, just missed this

Copy link
Contributor

@lforst lforst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup! Mild concerns about consistency but nothing blocking.

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) November 13, 2024 13:58
@AbhiPrasad AbhiPrasad merged commit e303818 into master Nov 13, 2024
11 checks passed
@AbhiPrasad AbhiPrasad deleted the abhi-re-org-js-integrations branch November 13, 2024 14:17
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants